-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[강원대 안드로이드 주민철] 6주차 스텝2 #78
base: joominchul
Are you sure you want to change the base?
[강원대 안드로이드 주민철] 6주차 스텝2 #78
Conversation
맵액티비티와 서치프래그먼트를 뷰 패키지로 이동시킴
맵액티비티와 서치프래그먼트를 뷰 패키지로 이동시킴
서비스에 있던 notification 관련 코드들을 따로 분리함
…-joominchul-step2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 이전 스텝에서 대부분 피드백 드린것 같아서 특별히 리뷰 드릴게 없네요.
사실 Hilt, Coroutine, Room 이 세 가지 기술스택을 6주만에 하나의 프로젝트에서 모두 체득화시키기에는 많이 힘드셨을 것 같습니다. 사실 그게 당연한 거기도 하구요. 마지막 주차이니 조금 첨언 드리자면 각각의 기술 스택에 대해서 심도깊은 학습의 시간을 가져봤으면 합니다. 사실 세 가지 기술 모두 이전의 힘들게 구현해야만 했던 과거의 불편함 (Hilt이면 Dagger를 이용한 의존성 관리, Coroutine의 경우엔 그간에 무수히 변화해온 비동기 프로그래밍 방식들, Room이면 Sqlite 직접 조작했던 과거)을 편리하게 구현하기 위해서 고안된 기술들이다 보니 많은 핵심 기능들이나 동작 방식들이 라이브러리 내에 숨겨져 있어서 당장 활용하는 사람 입장에서는 제대로 알고 사용하기가 쉽지 않습니다.
하지만 실무에서는 어떤 라이브러리를 사용할 떄 해당 라이브러리가 제공하는 API 하나하나 모두 동작 방식을 알고 사용해야 하는게 원칙이거든요. 그래서 이 기회를 잘 활용해서 각각의 기술들을 심층적으로 공부하신 후 제가 드린 코드랩이나 다른 잘 만든 레포를 참고하셔서 전체적으로 리팩토링을 진행해 보시면 좋겠습니다.
그리고 MVVM Pattern 이나 Repository Pattern에 대한 학습이 다소 미흡한 부분이 있는데 이 또한 같이 보강하시면 좋은 개발자가 되실거라 생각이 드네요.
아무튼 6주 동안 수고 많으셨고 또 다른 자리에서 뵈면 좋겠습니다.
감사합니다.
import androidx.core.app.NotificationCompat | ||
import campus.tech.kakao.map.view.SplashScreen | ||
|
||
class Notification { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네이밍을 좀 달리하면 좋겠네요. Notification을 띄우고 Notification Channel을 생성하는 거니 Notification ~~ (Manager 등) 으로 변경하심 좋겠습니다. 그리고 단순히 기능만 제공하니 object 클래스로 구현하셔도 괜찮을 것 같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
변경했습니다.
object NotificationModule { | ||
@Provides | ||
@Singleton | ||
fun provideNotification(): Notification { | ||
return Notification() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
의존성 주입까지 잘 하셨는데 사실 구현하신 Notification은 Service가 동작할 시에만 필요한 것이기에 굳이 의존성 주입 할 필요가 있나라는 생각도 드네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신 대로 굳이 필요한 것 같지 않아 삭제했습니다.
…-step2/android-map-notification into feat-joominchul-step2 # Conflicts: # README.md # app/src/androidTest/java/campus/tech/kakao/map/MapActivityUITest.kt # app/src/main/AndroidManifest.xml # app/src/main/java/campus/tech/kakao/map/view/SearchFragment.kt # app/src/main/java/campus/tech/kakao/map/view/SplashScreen.kt # app/src/main/java/campus/tech/kakao/map/viewModel/MainViewModel.kt # app/src/main/res/values/strings.xml # app/src/test/java/campus/tech/kakao/map/FunTest.kt
6주 동안 꼼곰한 피드백, 많은 도움이 되었습니다. 감사 인사 드리며 6주 동안 수고 많으셨습니다. |
어려웠던 점
6주 동안 수고 많으셨습니다!